Python: Fix flask request modeling#10629
Conversation
This takes us part of the way. We still get multiple paths for the same alert, but that will be fixed in a different PR.
tausbn
left a comment
tausbn
left a comment
There was a problem hiding this comment.
It's slightly strange to have the path start at the import, but I understand the reason for doing so.
I guess an alternative would be to not have request be a remote flow source in its own right, but only attributes thereof.
In the short term, I think this is a good fix, however. 👍
I didn't consider this, but I agree it could be a solution 🤔 |
|
Another alternative, which may amount to the same thing, is to say that importing the request object is not a case of remote input, but reading from it is. I am not sure if attribute reads is the only way to access it. |
It's really hard to audit that this is all good.. I tried my best with `icdiff` though -- and there is a problem with ql/src/experimental/Security/CWE-348/ClientSuppliedIpUsedInSecurityCheck.ql that needs to be fixed in the next commit
I realized the modeling was done in a non-recommended way, so I changed the modeling. It was very nice that I could use API graphs for the flask part, and a little sad when I couldn't for Django/Tornado.
|
this PR turned into quite a bit of a mixed bag now 😳 but everything was connected 🤷 |
3 participants
This takes us part of the way. We still get multiple paths for the same
alert, but that will be fixed in a different PR.